Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EMSUSD-490 Fixes outerliner not refreshing when performing a UsdUndoVisibleCommand #3345

Conversation

AramAzhari-adsk
Copy link
Collaborator

When undoing a visibility command, UsdNotice::ObjectsChanged does not produce a path for Info Change but still generates a path for Re-sync Change which has the Visibility token. The special case is added to refresh when in the latter path.

@AramAzhari-adsk
Copy link
Collaborator Author

AramAzhari-adsk commented Sep 25, 2023

It seems that when:

1- Making a prim invisible the following notifications are sent:
    AttributeChangeType::ValueAdded
    AttributeChangeType::ValueChanged

2- When setting the visibility back to "Inherited" / "Visible", only the following is sent which makes sense as the attribute already exists:
    AttributeChangeType::ValueChanged

3- When Undoing after manually making an item invisible, only the following notification is sent as the attribute will now be inheriting data:
    AttributeChangeType::ValueRemoved

This is checked here:

switch (changeType) {
case AttributeChangeType::kValueChanged: {
notifyWithoutExceptions<Ufe::Attributes>(
Ufe::AttributeValueChanged(ufePath, changedToken.GetString()));
if (UsdUfe::UsdCamera::isCameraToken(changedToken)) {
notifyWithoutExceptions<Ufe::Camera>(ufePath);
}
} break;
case AttributeChangeType::kAdded: {
if (UsdUfe::InSetAttribute::inSetAttribute()) {
notifyWithoutExceptions<Ufe::Attributes>(
Ufe::AttributeValueChanged(ufePath, changedToken.GetString()));
if (UsdUfe::UsdCamera::isCameraToken(changedToken)) {
notifyWithoutExceptions<Ufe::Camera>(ufePath);
}
} else {
notifyWithoutExceptions<Ufe::Attributes>(
Ufe::AttributeAdded(ufePath, changedToken.GetString()));
}
} break;
case AttributeChangeType::kRemoved: {
notifyWithoutExceptions<Ufe::Attributes>(
Ufe::AttributeRemoved(ufePath, changedToken.GetString()));
} break;

The bug is for #3. @seando-adsk Should we move the fix inside the     AttributeChangeType::ValueRemoved ?
I noticed there is a special case for the Camera in some of the AttributeChangeTypes

Comment on lines 398 to 403
// Special case when Undoing a visibility change, the notice.GetChangedInfoOnlyPaths()
// does not contain the change, hence handling visibility notification in re-synch path.
if (changedPath.GetNameToken() == UsdGeomTokens->visibility) {
Ufe::VisibilityChanged vis(ufePath);
notifyWithoutExceptions<Ufe::Object3d>(vis);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure the order matters, but I wonder if sending this extra visibilityChanged should come after the processAttributeChanges() just below. This would then match the order in the ChangedInfoOnlyPaths() loop.

Copy link
Collaborator Author

@AramAzhari-adsk AramAzhari-adsk Sep 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seando-adsk I actually had to move it as per this finding since having it in at line 398 caused duplicate notifications to be sent which failed the visibility test.

@AramAzhari-adsk AramAzhari-adsk added adsk Related to Autodesk plugin ready-for-merge Development process is finished, PR is ready for merge labels Sep 26, 2023
@AramAzhari-adsk
Copy link
Collaborator Author

One test on one platform did not pass which is a known random occurrence:
testVP2RenderDelegateUsdSkel

@AramAzhari-adsk AramAzhari-adsk added the bug Something isn't working label Sep 26, 2023
@seando-adsk seando-adsk merged commit e234252 into dev Sep 27, 2023
13 of 15 checks passed
@seando-adsk seando-adsk deleted the azharia/EMSUSD-490/fixes-outerliner-not-refreshing-when-performing-a-UsdUndoVisibleCommand branch September 27, 2023 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
adsk Related to Autodesk plugin bug Something isn't working ready-for-merge Development process is finished, PR is ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants